I'm now also thinking... these both seem a bit similar to ProdGrader; perhaps there's a better abstraction that would encompass both functionalities? ProdGrader just does weighted sums, but I wonder if it would make sense to parameterize it on some arbitrary combination operation. Then, OrGrader would just instantiate the more general abstraction at operation max; ConstraintGrader would instantiate with * maybe; what we have now would instantiate at a weighted average operation? (Could have others, too, like min, unweighted average, etc.) Thoughts?

Fair point. I think constraint grader would be more complex than * because it should give 0 points if the constraint is violated... but that would just a different function.

I'm not sure how useful some of those would be. Constraint grader has it's uses for offloading work from TAs/better organising some of the graders. The Or grader can be used for cases where we want to accept off-by-one errors, etc (bbs on lazy/imperative comes to mind). Definitely worth considering though.

I think constraint grader would be more complex than * because it should give 0 points if the constraint is violated... but that would just a different function.

I was thinking that if the constraint grader always gave 0 if the constraint were violated, it would be able to "knock out" the other score via multiplication by 0. It also works nicely with the style grader (which it'd be nice to express via the GRADER signature), since you could knock out only part of the correctness points.
Or, for a subtly-different behavior, constraint graders could use min instead of * - the zeroing-out behavior would stay the same, but the style grader wouldn't deduct points if the correctness was low anyway.

The design I was imagining is approximately as follows:

functor NewIdeaForProdGrader2 (
  val description : string  (* well, might want to rethink this per other discussion, but anyway *)
  structure Grader1 : GRADER
  structure Grader2 : GRADER
  val combine : (string * Rational.t) * (string * Rational.t) -> (string * Rational.t)
) = (* ... *)

Then:

functor CurrentProdGrader2 (
  val description : string
  structure Grader1 : GRADER
  structure Grader2 : GRADER
  val weights : int * int
) =
  NewIdeaForProdGrader2 (
    val description = description
    structure Grader1 = Grader1
    structure Grader2 = Grader2

    val (w1, w2) = weights
    val combine = fn ((s1, r1), (s2, r2)) => ((* format nicely *), weightedAverage [(r1, w1), (r2, w2)])
  )

functor OrGrader2 (
  val description : string
  structure Grader1 : GRADER
  structure Grader2 : GRADER
) =
  NewIdeaForProdGrader2 (
    val description = description
    structure Grader1 = Grader1
    structure Grader2 = Grader2
    val combine = fn ((s1, r1), (s2, r2)) =>
      case Rational.compare (r1, r2) of
        LESS => (s2, r2)
      | _    => (s1, r1)
  )

Ooooo yeah that would be quite nice.

I like the idea of moving the descriptions into the weights since that makes more sense to where they are used.

Would the style grader also just be a *? So if a student has 2 style errors, then the style grader would return 0.9 and we'd multiply that by whatever their actual score is.

Yeah, that's what I was thinking! :)

Sooo what's the tldr

I think to add to the conversation, I would prefer the constraint grader to not be just simple multiplication, since that is not how 150 grading works (at least not anymore). We deduct x amount of points, and if you reach below 0, we add it back so you get 0.

Oh in the case of memofib, we explicitly say we deduct 10 points (out of 15) for violating constraints, so would also like it if we could make it so that we can change the number of points we deduct

i think the tl;dr is that ideal the prod graders are rewritten to take in a score "combining" function which can be used to handle all these different use cases

Will this be worked on sometime in the near future?

It seems like it needs a review?

(I think you both should have the ability to review/merge, if it looks good to you!)

Also, should this be closed as superseded by #7, or both make sense?